fix(ui): guide legacy session account recovery - #4102

This is a synthesis of the independent blind review by @Sol-404ARE at exact head f15e7e4dbe8dc9df6a6e8d7bbaa7ced7ec6d6a86 (base 1975adb35caaa1dc8cd33b0bad75e34dcc90f592, 14 files +421/-40). I verified the diff and the exact-head CI myself; the file:line findings below are from Sol's sealed review.

What I checked myself:

• Read gh pr diff 4102 (fix: guide legacy session account recovery, 14 files) and confirmed the recovery routing through use-shell-chat-model.ts:231-255, session-health-notice.ts:125-143, and the picker freshness at use-shell-connections.ts:92-129 / chat-model-switcher.tsx:245-249,282-303.
• Checked exact-head CI: test SUCCESS, label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's review (file:line anchored):

Standards — NO-GO — 3×P2 + 1×P3 (worst P2)

1. P2 — Required PR template was replaced — CONTRIBUTING.md:81-87 requires the template's AI-selection/tool-scope (:25-36), checklist (:38-41), and behavior selector (:43-46); the body omits all three. This is counted as one systemic hygiene finding.

2. P2 — Required UI evidence is absent — visible CTA/picker behavior changes at chat-message-surface.tsx:290-305, app-shell.tsx:760-763,2944-2949, and composer.tsx:1968-1981 have no before/after screenshot or recording, violating CONTRIBUTING.md:87.

3. P2 — No composed oracle for notice CTA → hook routing → AppShell ref adapter → controlled Composer picker — renderer/UI standards at renderer/README.md:78-83 and ui/README.md:58-64 require a focused behavioral contract. Existing tests separately check pure target derivation and direct imperative picker opening, so deleting the Banner click binding, the hook routing, or the AppShell adapter can leave them green. Add a composed CTA → picker/reload/settings contract plus the live-turn-disabled case.

4. P3 — Judgment-only Duplicated Code / authority drift — app-shell.tsx:890-896 reconstructs switch safety while composer.tsx:1359-1378,1977-1978,2014-2016 separately derives lock/reason and pending state; prefer a single availability witness/reason contract. Other Fowler concerns were withdrawn; entropy rises slightly from the duplicated switch-lock authority.

No other Standards finding; EN/zh and architecture boundaries pass.

Spec — NO-GO — 1×P2 (worst P2)

• P2 — A failed catalog refresh can leave the deleted account as a normal checked recovery choice whose selection is a no-op — use-shell-connections.ts:92-129,140-145 retains the prior snapshot after getSnapshot fails and still reports hasSnapshot=true. use-shell-chat-model.ts:231-255 then treats the choice list as settled/non-empty, and session-health-notice.ts:125-143 routes straight to the picker. In chat-model-switcher.tsx:245-249,282-303 the stale exact choice A remains checked; selecting it returns early at :293-297 and the Host performs no validation/mutation. Repro: load Session A with a ready choice, delete A, make the connection_list_changed refresh fail, then observe connection_missing; clicking the CTA and selecting the still-checked A is a no-op and the Session stays blocked. The existing test at composer-model-picker-recovery.test.tsx:64-111 only uses a fresh replacement plus a legacy Session without id, so this stale-exact-A + failing-refresh case is missed. Invalidate or visibly disable stale choices and force a refresh/revalidation before accepting the current exact choice.

Spec exclusions that were verified as green: TUI correctly limits /setup to API-key-only; /model persists exact {model,slug,id}; Desktop Host mutation is exact; switch locks/fresh picker and duplicate-name disambiguation pass; no stale missing-llmConnectionId memo bug.

Verification: git diff --check PASS, targeted Biome 14/14 PASS, ASF headers 14/14 PASS, worktree clean, head unchanged, test/label SUCCESS.

What I did not judge: a true picker refresh-failure → stale-choice → no-op E2E with a real Host/catalog was not executed beyond race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 3×P2 and Spec 1×P2 remain; despite test/label green, head f15e7e4 is not merge-ready as “guide legacy session account recovery” until the stale-choice invalidation and the UI evidence/template are closed. Seal: notes/pr-4102-provisional.md.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

This is a synthesis of the feature-gate audit by @feature杀手 at exact head 775f39b03e3dfd80c87dead8ef2542b3e03f6bb2 (increment f15e7e4 → 775f39b). I verified the drift and the increment myself; the file:line findings below are from the auditor's sealed report.

What I checked myself:

• Compared f15e7e4 → 775f39b (delta for the new repair, ~+166 lines plus 775f39b guard) and confirmed the hidden-Composer picker path at app-shell.tsx:760-763,890-896,2871-2873 / composer.tsx:1572-1575 and the side-conversation ID check at use-quote-companion.ts:475-480,536-543 / session-manager.ts:4981-4989.
• Checked head state: 775f39b03e3d80c87dead8ef2542b3e03f6bb2 is now the remote head (drift from f15e7e4), test pending at last check, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from the feature audit (file:line anchored, notes/pr-4102-feature-audit.md):

Overall — Necessity GO, existing Host authority design GO; CODE / GLOBAL-DESIGN NO-GO — not merge-ready.

• Blocker 1 — The recovery button is clickable while the Composer that owns the picker is hidden. app-shell.tsx:890-896 computes modelPickerDisabled without localInteractionAvailable, but :2871-2873 hides the Composer when localInteractionAvailable=false. Clicking still reaches :760-763 and sets the menu state to open, while the menu lives inside the native hidden form at composer.tsx:1572-1575. The exact-head probe shows actionDisabled=false / formHidden=true / menuOpenedInsideHiddenForm=true — the user sees “Select connection and model,” clicks, and nothing visible happens; the pre-opened menu can later pop out when the boundary restores. The picker availability should be a single shared fact that gates both the banner CTA and the Composer — hide means disabled/rejected, without adding a new recovery state.

• Blocker 2 — The new side-conversation defense only looks at llmConnectionId presence. use-quote-companion.ts:475-480,536-543 only blocks legacy null-ID; a deleted Connection or an ID/slug mismatch still carries a non-empty ID and will immediately fork. The Runtime then copies the old ID/slug/model at session-manager.ts:4981-4989, so a child forked from a stale source is not repaired when the source later recovers, and the side conversation has no independent picker. The probe proves deleted-connection-id is forked once and retained. New tests only cover undefined-ID. The fork/initial-send should reuse the existing Host/catalog exact source-configuration readiness check and cover at least legacy_connection_identity, connection_missing, and connection_identity_mismatch before forking, without building a second side-chat identity state machine.

Globally it is correct not to split the PR and not to add a new Runtime Host operation/wire/epoch/durable migration: the existing exact {connectionId, connectionSlug, model} mutation is the single correct write authority, and the new ChatRecoveryNotice does converge three previously duplicated Banner layouts. The right simplification is to converge the two partial readiness predicates above. After code closure the PR body should also be updated to the true 3 commits / 20 files (currently missing the side-conversation defer and notice convergence, and “two pre-PR reviews” does not describe the new head).

Verification: Desktop 1669/1669, UI 248/248, TUI focused 2/2, three-workspace typecheck, 20-file Biome and diff-check all PASS; two adversarial probes stably reproduce the gaps. The sole CLI failure #4011 was isolated and passed on retry. Remote test was pending at last recheck. No GitHub writes were made prose.

Gate: two P-level blockers remain on the increment f15e7e4 → 775f39b; despite the prior head f15e7e4 having been CODE GO, the current head is not merge-ready until the hidden-picker and side-conversation readiness predicates are converged. Prior head comment 5455485290 applies to f15e7e4 only and must not be extrapolated.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

This is a synthesis of the independent blind review by @Sol-404ARE at exact head 1f4e367bf67c2fa89ffde8480bf1af8bd461fd8f (base 1975adb35caaa1dc8cd33b0bad75e34dcc90f592, 27 files +1674/-139). I verified the drift and the exact-head CI myself; the file:line findings below are from Sol's sealed bounded freshness review against f15e7e4.

What I checked myself:

• Read the incremental diff f15e7e4 → 1f4e367 (5 commits / 20 files +1281/-127, stale-snapshot and side-chat fork lifecycle) and confirmed the fixes at use-shell-connections.ts:93-103,118-148 (+ seedSnapshot guard :87-90), quote-companion-core.ts:60-72 / use-quote-companion.ts:485-615 / quote-companion-panel.tsx:133-168,245-271, and the composed adapters at composer-model-picker-adapter.ts:24-30 + session-health-recovery-flow.test.ts:86-217.
• Checked exact-head CI: test run 33196509253 QUEUED, label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's bounded review:

Standards — NO-GO — 2×P2 + 3×P3 (worst P2)

• Closed: prior composed-oracle P2 — session-health-recovery-flow.test.ts:86-217 now combines the production hook, useComposerModelPickerAdapter, recovery notice, and controlled Composer to cover CTA → picker, reload, Settings, and live-turn lock, satisfying renderer/README.md:78-83 and ui/README.md:58-64; composer-model-picker-adapter.ts:24-30 makes the AppShell ref seam directly testable.

• Remaining P2 — PR template is replaced and the body is stale — CONTRIBUTING.md:81-87 requires the template's AI-selection/tool-scope (:25-36), checklist (:38-41), and behavior selector (:43-46); the body still omits all three. The Summary/Validation also still describe the old 14-file/18-test patch and do not mention the stale-snapshot and side-chat lifecycle increments. Counted as one metadata root; five new empty commit bodies are not separately counted.

• Remaining P2 — UI media is still absent — the new shared chat-recovery-notice.tsx:25-69, delegated notices in chat-message-surface.tsx, and wrapper styles in chat-header.css:30-51 all change visible surfaces, yet the body has no before/after screenshot/recording required by CONTRIBUTING.md:87.

• Remaining P3 — judgment-only Duplicated Code/authority drift — app-shell.tsx:889-895 rebuilds model-switch safety while composer.tsx:1359-1378,1968-1981,2009-2016 separately derives lock/reason/pending gate; prefer a single availability witness/reason.

• New P3 — judgment-only Primitive Obsession/Data Clump — projection lifecycle is encoded as Map absence vs null plus two booleans hasSnapshot/hasProjectionState in use-shell-connections.ts:63-80,99-117,145-152 and decoded in app-shell.tsx:552-562; prefer a discriminated unrequested|refreshing|ready.

• New P3 — judgment-only test Duplicated Code — quote-companion-retry.test.ts grows 463 lines to 1,645 with repeated sourceA/sourceB/forkB/provider rerender fixtures at :386-439,485-546,566-630; extract a scenario builder.

No new layer violation was found; Shotgun Surgery remains excluded. Entropy is mixed: the shared notice plus composed oracle reduce entropy, but boolean state and repeated fixtures add some.

Spec — GO — 0 P0–P3

• Closed: prior stale exact choice/no-op P2 — use-shell-connections.ts:93-103 now nulls the Host projection before each refresh; on failure it stays unsettled with hasSnapshot=false (:118-148), and the existing key blocks seedSnapshot from replanting stale data (:87-90). A deleted choice A therefore never enters the picker; identity-recovery failures now route to model_choices_refresh (session-health-notice.ts:121-143) and only a successful current Host read exposes a replacement.

• Side-chat is now fail-closed on the exact {connectionId,slug,model} triple (quote-companion-core.ts:60-72; use-quote-companion.ts:485-615), re-validated after async create, without replacing existing content/admission/turn forks; source/fork non-current blocks send at quote-companion-panel.tsx:133-168,245-271. Race tests cover unavailable source, stale empty fork, in-flight create, observation wait, admission, and interrupted turns (:347-735).

• The transient fail-closed refresh will temporarily hide switching/side-chat, but it requires an explicit Reload and never exposes a stale mutation target — a safe tradeoff. Rebuilding an empty ephemeral fork does not silently rebind a durable Session. No new Spec regression or scope creep was found.

Verification: incremental/full git diff --check PASS, targeted Biome 24/24 PASS, ASF audit PASS, Astryx inventory 219/1 PASS, worktree clean, head unchanged, test QUEUED (so terminal CI not yet proven). Remote test had been SUCCESS on the fixed head f15e7e4; the new increment awaits its test run.

What I did not judge: a true picker refresh-failure → stale-choice → no-op E2E with a live Host was not executed beyond race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 2×P2 + 3×P3 remain, Spec GO — despite the new test run being QUEUED, the head is not merge-ready until the template/body and UI media are closed (Spec is ready). Seal: notes/pr-4102-1f4e-freshness.md.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.

This is a synthesis of the independent blind review by @Sol-404ARE at exact head cfda0cb0e544827d3fb171a30012a570ad1f132b (base 1975adb35caaa1dc8cd33b0bad75e34dcc90f592, 28 files +1768/-166). I verified the drift and the exact-head CI myself; the file:line findings below are from Sol's sealed bounded freshness review against 1f4e367.

What I checked myself:

• Read the incremental diff 1f4e367 → cfda0cb (2 commits 762e537 + cfda0cb, 9 files +228/-161, clarifying recovery state contracts and consolidating side-chat race fixtures) and confirmed the fixes at use-shell-connections.ts, deriveComposerModelSwitchAvailability, and quote-companion helpers.
• Checked exact-head CI: test run 33198982319 FAILURE — Desktop E2E prompt-rail.spec.ts:288 timeout waiting for [data-virtual-turn-id="turn-prompt-rail-120"]; label SUCCESS, OPEN/MERGEABLE/BLOCKED/REVIEW_REQUIRED.

Findings from Sol's bounded review:

Standards — NO-GO — 1×P2 (worst P2)

• Closed: all five prior findings are now closed: the body now follows the required template and is current; two exact-head production Electron screenshots satisfy CONTRIBUTING.md:87 for UI-evidence; deriveComposerModelSwitchAvailability is now the shared typed authority used by AppShell, recovery notice, Composer controls, and imperative picker opening; projection lifecycle is now the explicit unrequested | refreshing | ready union; side-chat race fixtures are consolidated without losing scenarios. Entropy materially decreases.

• New P2 — AI disclosure conflicts with commit trailers. The body states OpenAI Codex substantively assisted code, tests, and review analysis. CONTRIBUTING.md:32-36 therefore requires Generated-by: <tool> trailers on every affected commit and retention through squash/amend. All eight PR commits (f15e7e4, 0a663a7, 775f39b, ae937bb, c8da4d2, 1f4e367, 762e537, cfda0cb) have empty bodies and no trailers. Amend/rewrite the affected commits with Generated-by: OpenAI Codex (or precisely identify any human-only commits) and preserve the trailer in the squash commit.

Spec — GO — 0 P0–P3

• The discriminated projection preserves invalidation-before-read, fail-closed failed-refresh behavior, stale-seed rejection, sequence/current-Host guards, and only treats ready as settled; stale deleted-account choices therefore cannot reappear. The centralized switch contract behavior-equivalently gates the recovery CTA, Composer controls, and imperative picker; exact Host persistence remains the full {connectionId, connectionSlug, model} mutation. Fixture consolidation retains setup, empty replacement, observation wait, pending admission, admitted content/live turn, stale fork, and exact ID/slug/model assertions. No TUI behavior changed; no missing/partial/wrong behavior or scope creep was found.

Verification: incremental/full git diff --check PASS, targeted Biome 25/25 PASS, Astryx inventory 219/1 PASS, ASF header audit PASS, worktree clean, head unchanged.

What I did not judge: a true picker refresh-failure → stale-choice → no-op E2E with a live Host was not executed beyond race-inspection — verification was by code inspection and the tests noted above.

Gate: Standards 1×P2 (attribution) remains; despite Spec GO, head cfda0cb is not merge-ready until the Generated-by trailers are added and the exact-head test is terminal green (current run fails on an unrelated prompt-rail E2E). Seal: notes/pr-4102-cfda-freshness.md.

Automated review notice: This comment was posted by an automated review agent operated by Astro-Han. It is not an independent human review and does not replace one.